Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement remove_tokens_for_client() #666

Merged
merged 1 commit into from
Feb 21, 2024
Merged

Conversation

rayluo
Copy link
Collaborator

@rayluo rayluo commented Feb 12, 2024

Based on this understanding, here comes this (evil?) PR, which will resolve #640 and resolve #650

@rayluo rayluo merged commit 3b96de6 into dev Feb 21, 2024
12 checks passed
@rayluo rayluo deleted the remove-tokens-for-client branch February 21, 2024 19:13
@rayluo rayluo mentioned this pull request Feb 22, 2024
@jiasli
Copy link
Contributor

jiasli commented Mar 20, 2024

here comes this (evil?) PR

Interestingly, 666 is a positive/luckily number in Chinese, expressing "good" and "proficient": https://en.wikipedia.org/wiki/666_(number).

@yonzhan
Copy link

yonzhan commented Mar 20, 2024 via email

@jiasli
Copy link
Contributor

jiasli commented Jul 22, 2024

The implementation is not complete. After running remove_tokens_for_client(), the ghost of the service principal still lingers in AppMetadata:

{
    "AccessToken": {},
    "AppMetadata": {
        "appmetadata-login.microsoftonline.com-a7003d8c-e50f-4371-91a6-ef37bba4ab23": {
            "client_id": "a7003d8c-e50f-4371-91a6-ef37bba4ab23",
            "environment": "login.microsoftonline.com"
        }
    }
}

Comment on lines +2186 to +2190
for at in self.token_cache.find(TokenCache.CredentialType.ACCESS_TOKEN, query={
"client_id": self.client_id,
"environment": env,
"home_account_id": None, # These are mostly app-only tokens
}):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Luckily realm (tenant) is not used in query. This makes the functionality align with az logout which only takes a --username, but not --tenant.

This means all access tokens for a service principal from all tenants will be removed. However, this brings up another question: what if I only want to remove the access token for a service principal in a specific tenant?

@jiasli
Copy link
Contributor

jiasli commented Jul 22, 2024

What if I lose track of service principals client IDs? How do I purge all service principals' access tokens, while keeping users' access tokens?

@rayluo
Copy link
Collaborator Author

rayluo commented Aug 2, 2024

What if I lose track of service principals client IDs? How do I purge all service principals' access tokens, ...?

Hmm, MSAL's ClientApplication always requires client_id as its required parameter. This question sounds as strange as "what if I forgot my local Windows username, how do I purge all users' content from the current PC?". Is there a legit scenario for that? That being said, I suppose you can do format c: or its equivalent rm ~/.azure/msal_token_cache.json if desirable.

How do I purge all service principals' access tokens, while keeping users' access tokens?

You would have to create different ClientApplication objects with different token cache persistence files, so that you may be able to delete some of those files.

@rayluo
Copy link
Collaborator Author

rayluo commented Aug 2, 2024

The implementation is not complete. After running remove_tokens_for_client(), the ghost of the service principal still lingers in AppMetadata:

{
    "AccessToken": {},
    "AppMetadata": {
        "appmetadata-login.microsoftonline.com-a7003d8c-e50f-4371-91a6-ef37bba4ab23": {
            "client_id": "a7003d8c-e50f-4371-91a6-ef37bba4ab23",
            "environment": "login.microsoftonline.com"
        }
    }
}

The residue of AppMetadata is technically not an incomplete implementation. The AppMetadata was meant to be persisted forever, because (1) they would be used to remember whether an app belongs to a "family" (although that characteristic does not necessarily apply to service principal); (2) they contain no sensitive tokens anyway.

@jiasli
Copy link
Contributor

jiasli commented Aug 5, 2024

This question sounds as strange as "what if I forgot my local Windows username, how do I purge all users' content from the current PC?".

This is possible for user accounts logged into MSAL as users can be retrieved with ClientApplication.get_accounts():

def get_accounts(self, username=None):

Azure CLI has a logout_all_users() which removes all users, but logout_service_principal() is currently incomplete due to no equivalent of ClientApplication.get_accounts().

@jiasli
Copy link
Contributor

jiasli commented Aug 5, 2024

although that characteristic does not necessarily apply to service principal

I agree the client ID for user authentication (such as Azure CLI's client ID) is not sensitive, but some users may treat service principal's client ID as sensitive date and want it to be "obliviated".

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Feature Request] Support force_refresh for service principal Feature Request: Removing app tokens
4 participants